st/widget: Don't animate theme node updates on theme context changes

With commit 4dfa39457e, I was assuming that emitting `::changed` on the
StThemeContext, triggering StWidgets recursive style-changed machinery,
is enough to get rid of all the theme nodes that are in use, ensuring
they're never accessed again after the `::changed` emission.

That assumption turned out to be wrong: `st_widget_recompute_style()`
animates updates of theme nodes using StThemeNodeTransition, and
StThemeNodeTransition keeps the old theme node around, accessing it at
various later points. This caused various crashes on extension
loading/unloading, because that triggers a theme context change, where
a StThemeNodeTransition would later access an outdated theme node,
pointing to CSS properties that are already freed.

That said, reverting 4dfa39457e doesn't seem like the best idea,
because other calls to `st_theme_context_changed()` also seem to make
the same (incorrect) assumption, eg. `st_theme_context_set_theme()`
seems like a good candidate for a similar crash.

Probably St just shouldn't try to animate on theme context changes: The
old theme nodes are no longer usable after those, but
StThemeNodeTransition expects them to be, so that's just a bad idea.

So change the style-changed machinery a bit to allow disabling transitions
and use that to avoid transitions on theme context changes.

Fixes: 4dfa39457e ("st/theme-context: Also remove theme nodes on stylesheet changes")
Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/7339
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3809>

diff --git a/src/st/st-widget.c b/src/st/st-widget.c
index ef96e2713307a36eed4a21e25cdb21a27d080dd8..1299b1cb5ba2ff7bf6fd5f733c18e568033949dd 100644 (file)
--- a/src/st/st-widget.c
+++ b/src/st/st-widget.c
@@ -128,8 +128,15 @@ static guint signals[LAST_SIGNAL] = { 0, };
 G_DEFINE_TYPE_WITH_PRIVATE (StWidget, st_widget, CLUTTER_TYPE_ACTOR);
 #define ST_WIDGET_PRIVATE(w) ((StWidgetPrivate *)st_widget_get_instance_private (w))
 
-static void st_widget_recompute_style (StWidget    *widget,
-                                       StThemeNode *old_theme_node);
+typedef enum {
+  STYLE_CHANGE_FLAGS_NONE = 0,
+  STYLE_CHANGE_FLAGS_NO_TRANSITIONS = 1 << 0,
+} StyleChangeFlags;
+
+static void st_widget_recompute_style (StWidget         *widget,
+                                       StThemeNode      *old_theme_node,
+                                       StyleChangeFlags  flags);
+
 static gboolean st_widget_real_navigate_focus (StWidget         *widget,
                                                ClutterActor     *from,
                                                StDirectionType   direction);
@@ -484,7 +491,12 @@ st_widget_unmap (ClutterActor *actor)
 }
 
 static void
-notify_children_of_style_change (ClutterActor *self)
+st_widget_style_changed_internal (StWidget         *widget,
+                                  StyleChangeFlags  flags);
+
+static void
+notify_children_of_style_change (ClutterActor     *self,
+                                 StyleChangeFlags  flags)
 {
   ClutterActorIter iter;
   ClutterActor *actor;
@@ -493,9 +505,9 @@ notify_children_of_style_change (ClutterActor *self)
   while (clutter_actor_iter_next (&iter, &actor))
     {
       if (ST_IS_WIDGET (actor))
-        st_widget_style_changed (ST_WIDGET (actor));
+        st_widget_style_changed_internal (ST_WIDGET (actor), flags);
       else
-        notify_children_of_style_change (actor);
+        notify_children_of_style_change (actor, flags);
     }
 }
 
@@ -505,8 +517,9 @@ st_widget_real_style_changed (StWidget *self)
   clutter_actor_queue_redraw ((ClutterActor *) self);
 }
 
-void
-st_widget_style_changed (StWidget *widget)
+static void
+st_widget_style_changed_internal (StWidget         *widget,
+                                  StyleChangeFlags  flags)
 {
   StWidgetPrivate *priv = st_widget_get_instance_private (widget);
   StThemeNode *old_theme_node = NULL;
@@ -520,22 +533,28 @@ st_widget_style_changed (StWidget *widget)
 
   /* update the style only if we are mapped */
   if (clutter_actor_is_mapped (CLUTTER_ACTOR (widget)))
-    st_widget_recompute_style (widget, old_theme_node);
+    st_widget_recompute_style (widget, old_theme_node, flags);
 
   /* Descend through all children. If the actor is not mapped,
    * children will clear their theme node without recomputing style.
    */
-  notify_children_of_style_change (CLUTTER_ACTOR (widget));
+  notify_children_of_style_change (CLUTTER_ACTOR (widget), flags);
 
   if (old_theme_node)
     g_object_unref (old_theme_node);
 }
 
+void
+st_widget_style_changed (StWidget *widget)
+{
+  st_widget_style_changed_internal (widget, STYLE_CHANGE_FLAGS_NONE);
+}
+
 static void
 on_theme_context_changed (StThemeContext *context,
                           ClutterStage   *stage)
 {
-  notify_children_of_style_change (CLUTTER_ACTOR (stage));
+  notify_children_of_style_change (CLUTTER_ACTOR (stage), STYLE_CHANGE_FLAGS_NO_TRANSITIONS);
 }
 
 static StThemeNode *
@@ -1638,8 +1657,9 @@ on_transition_completed (StThemeNodeTransition *transition,
 }
 
 static void
-st_widget_recompute_style (StWidget    *widget,
-                           StThemeNode *old_theme_node)
+st_widget_recompute_style (StWidget         *widget,
+                           StThemeNode      *old_theme_node,
+                           StyleChangeFlags  flags)
 {
   StWidgetPrivate *priv = st_widget_get_instance_private (widget);
   StThemeNode *new_theme_node = st_widget_get_theme_node (widget);
@@ -1661,7 +1681,9 @@ st_widget_recompute_style (StWidget    *widget,
   if (!geometry_equal)
     clutter_actor_queue_relayout ((ClutterActor *) widget);
 
-  transition_duration = st_theme_node_get_transition_duration (new_theme_node);
+  transition_duration = (flags & STYLE_CHANGE_FLAGS_NO_TRANSITIONS) != 0
+    ? 0
+    : st_theme_node_get_transition_duration (new_theme_node);
 
   paint_equal = st_theme_node_paint_equal (old_theme_node, new_theme_node);
 
@@ -1736,8 +1758,8 @@ st_widget_ensure_style (StWidget *widget)
 
   if (priv->is_style_dirty)
     {
-      st_widget_recompute_style (widget, NULL);
-      notify_children_of_style_change (CLUTTER_ACTOR (widget));
+      st_widget_recompute_style (widget, NULL, STYLE_CHANGE_FLAGS_NONE);
+      notify_children_of_style_change (CLUTTER_ACTOR (widget), STYLE_CHANGE_FLAGS_NONE);
     }
 }